Skip to content

fix(settings): correctly detect Chrome on Android in devices & sessions#58739

Open
chandrika1993 wants to merge 1 commit intonextcloud:masterfrom
chandrika1993:fix/issue-50502
Open

fix(settings): correctly detect Chrome on Android in devices & sessions#58739
chandrika1993 wants to merge 1 commit intonextcloud:masterfrom
chandrika1993:fix/issue-50502

Conversation

@chandrika1993
Copy link

@chandrika1993 chandrika1993 commented Mar 5, 2026 •
edited
Loading

Summary

Chrome on Android was incorrectly displayed as Google Chrome (Linux) in the
Devices & Sessions list at /settings/user/security.

Two compounding issues caused this:

  1. Android Chrome UA strings contain Linux (Android is Linux-based), so the
    generic chrome regex matched before androidChrome was ever reached.
  2. Since 2021, Google reduced Android Chrome's UA — the Build/ token is no
    longer included, so the old androidChrome regex failed to match modern
    Android Chrome entirely.

Fix:

  • Updated androidChrome regex to match both modern (no Build/) and legacy
    (with Build/) Android Chrome UA formats
  • Added negative lookahead (?![^)]*Android) to chrome and firefox regexes
  • Moved androidChrome before chrome in userAgentMap as defense in depth

Before / After:

Before After
Label Google Chrome - 132 (Linux) ❌ Google Chrome for Android - 132 ✅
Icon 🌐 Chrome 🤖 Android

❌ Before (Bug)

old-bug

✅ After (Fixed)

bug-fix

Note: This PR supersedes #56450 which addresses the modern UA format
but does not fix the underlying map ordering issue that would still
allow the chrome regex to match Android UAs first. This PR combines
both fixes for a complete solution.

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit) are included — 13 tests added and passing
  • Screenshots before/after included above
  • Documentation not required
  • Backports requested where applicable
  • Labels added — bug, feature: settings, 3. to review
  • Milestone added

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

cc @skjnldsv @provokateurin @sorbaugh @nfebe

@chandrika1993 chandrika1993 requested a review from a team as a code owner March 5, 2026 16:11
@chandrika1993 chandrika1993 requested review from artonge, skjnldsv and szaimen and removed request for a team March 5, 2026 16:11
@chandrika1993 chandrika1993 mentioned this pull request Mar 5, 2026
Open
8 tasks
@artonge artonge added this to the Nextcloud 34 milestone Mar 10, 2026
artonge
artonge reviewed Mar 10, 2026
View reviewed changes
Copy link
Collaborator

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for the PR.

Can you manually review your tests cases to ensure that they only test the result of the detect function and not its implementation. And also ensure that each test case actually make sense?

Please also address the other comments.

@chandrika1993 chandrika1993 requested review from ArtificialOwl, artonge and leftybournes and removed request for a team March 10, 2026 19:45
@chandrika1993 chandrika1993 force-pushed the fix/issue-50502 branch 2 times, most recently from 0f234dc to a17dd64 Compare March 10, 2026 20:02
@chandrika1993
Copy link
Author

chandrika1993 commented Mar 10, 2026

Hi @artonge,

Thanks for the review. I have addressed all the suggested changes. Let me know if anything else is needed.

Also, apologies for the noise, I accidentally rebased my branch with master, which triggered auto-assignment of several reviewers. I've since reverted to the original state. Could you help remove the extra reviewers?

Best,
Chandrika

@miaulalala
Copy link
Contributor

miaulalala commented Mar 12, 2026

npm is still not right, please run the commands from above, commit all the resulting changes and amend your last commit before you force push

@chandrika1993
Copy link
Author

chandrika1993 commented Mar 12, 2026

npm is still not right, please run the commands from above, commit all the resulting changes and amend your last commit before you force push

Please check I did the requested

@miaulalala
Copy link
Contributor

miaulalala commented Mar 16, 2026

npm is still not right, please run the commands from above, commit all the resulting changes and amend your last commit before you force push

Please check I did the requested

you need to commit the files in the /js/ folder - js.min and js.min.map files

@chandrika1993
Copy link
Author

chandrika1993 commented Mar 16, 2026

npm is still not right, please run the commands from above, commit all the resulting changes and amend your last commit before you force push

Please check I did the requested

you need to commit the files in the /js/ folder - js.min and js.min.map files

Hi, I ran npm ci && npm run build but the build is not generating any .min.js files. The build output goes to the dist/ folder as .js files (e.g. dist/settings-vue-settings-personal-security.js) which are already minified and committed. Could you point me to the exact build command that generates .min.js files, or confirm if the dist/ files I've already committed are sufficient?

artonge
artonge reviewed Mar 18, 2026
View reviewed changes
@miaulalala
Copy link
Contributor

miaulalala commented Mar 18, 2026

npm is still not right, please run the commands from above, commit all the resulting changes and amend your last commit before you force push

Please check I did the requested

you need to commit the files in the /js/ folder - js.min and js.min.map files

Hi, I ran npm ci && npm run build but the build is not generating any .min.js files. The build output goes to the dist/ folder as .js files (e.g. dist/settings-vue-settings-personal-security.js) which are already minified and committed. Could you point me to the exact build command that generates .min.js files, or confirm if the dist/ files I've already committed are sufficient?

ah, sorry, run the commands within the apps/settings folder then you should see the JS changes.

@chandrika1993 chandrika1993 force-pushed the fix/issue-50502 branch 3 times, most recently from e81d75f to 65316c3 Compare March 18, 2026 15:08
miaulalala
miaulalala approved these changes Mar 18, 2026
View reviewed changes
Copy link
Contributor

@miaulalala miaulalala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@artonge artonge mentioned this pull request Mar 18, 2026
Draft
@artonge artonge enabled auto-merge March 18, 2026 16:58
@artonge
Copy link
Collaborator

artonge commented Mar 19, 2026

/compile rebase /

@miaulalala
Copy link
Contributor

miaulalala commented Mar 19, 2026

@chandrika1993 can you allow maintainer changes? Then we can run the nextcloud bot that will compile the JS again.

@artonge
Copy link
Collaborator

artonge commented Mar 19, 2026

/compile rebase /

@miaulalala
Copy link
Contributor

miaulalala commented Mar 19, 2026

/compile amend

@miaulalala
Copy link
Contributor

miaulalala commented Mar 19, 2026

@chandrika1993 you need to solve the conflicts before we can merge

Please run a rebase. The conflict on the dist folder you can resolve in any way you like as they'll be regenerated anyway.

After you've run the rebase and resolved the conflicts, rerun npm ci && npm run build in the settings app, add the new generated files in the /dist/ folder and amend you last commit.

auto-merge was automatically disabled March 19, 2026 16:37

Head branch was pushed to by a user without write access

@chandrika1993 chandrika1993 force-pushed the fix/issue-50502 branch 2 times, most recently from c279871 to c3864a5 Compare March 19, 2026 18:07
@chandrika1993
Copy link
Author

chandrika1993 commented Mar 19, 2026

@chandrika1993 you need to solve the conflicts before we can merge

Please run a rebase. The conflict on the dist folder you can resolve in any way you like as they'll be regenerated anyway.

After you've run the rebase and resolved the conflicts, rerun npm ci && npm run build in the settings app, add the new generated files in the /dist/ folder and amend you last commit.

The branch is updated now

@chandrika1993
fix(settings): correctly detect Chrome on Android in devices & sessions
3ec0b55 
Signed-off-by: Chandrika Mohan <chandrikalov@gmail.com>

common logic to detect ua

Signed-off-by: Chandrika Mohan <chandrikalov@gmail.com>
@miaulalala
Copy link
Contributor

miaulalala commented Mar 19, 2026

you need to compile the js code again

@github-actions
Copy link
Contributor

github-actions bot commented Mar 20, 2026

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@artonge artonge artonge approved these changes

@miaulalala miaulalala miaulalala approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv skjnldsv is a code owner automatically assigned from nextcloud/server-frontend

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@provokateurin provokateurin Awaiting requested review from provokateurin

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@SebastianKrupinski SebastianKrupinski Awaiting requested review from SebastianKrupinski

@tcitworld tcitworld Awaiting requested review from tcitworld

@hamza221 hamza221 Awaiting requested review from hamza221

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@leftybournes leftybournes Awaiting requested review from leftybournes leftybournes was automatically assigned from nextcloud/server-backend

Assignees

No one assigned

Labels

3. to review Waiting for reviews AI assisted feature: settings feedback-requested javascript

Projects

None yet

Milestone

Nextcloud 34

Development

Successfully merging this pull request may close these issues.

[Bug]: In /settings/user/security Chrome on Android is detected as Chrome on Linux

3 participants

@chandrika1993 @miaulalala @artonge